Skip to content

Expose reply_path from Responder in onion messenger #3906

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed

Conversation

a-mpch
Copy link
Contributor

@a-mpch a-mpch commented Jul 2, 2025

Context

When using responder from OnionMessenger and using an external process to interact with the messages (our case in LNDK, using LND) we need to try to connect to the introduction node before responding to the message itself.

What it is implemented

Just exposing the reply path from the responder.

@ldk-reviews-bot
Copy link

ldk-reviews-bot commented Jul 2, 2025

👋 I see @valentinewallace was un-assigned.
If you'd like another reviewer assignment, please click here.

@ldk-reviews-bot ldk-reviews-bot requested a review from wpaulino July 2, 2025 17:53
@wpaulino wpaulino requested review from valentinewallace and removed request for wpaulino July 3, 2025 02:10
@@ -413,6 +413,11 @@ impl Responder {
Responder { reply_path }
}

/// Returns the reply path for this [`Responder`].
pub fn reply_path(&self) -> BlindedMessagePath {
self.reply_path.clone()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure I understand why this needs to be cloned. Can't we return a reference?

Copy link
Contributor Author

@a-mpch a-mpch Jul 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, I thought it would be safer not returning a reference. (stuck on C, my bad, is not mutable)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No worries! Regarding needing a direct connection, when sending a message over the reply path with OnionMessenger it should buffer the message if it needs a connection with the introduction node. Then you can use OnionMessenger::process_pending_events to process any ConnectionNeeded events. Once connected and OnionMessenger::peer_connected is called, the buffered message will be released upon calling OnionMessenger::next_onion_message_for_peer.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh I see, thank you!
I'd need to do a bigger refactor than I thought. We aren't using events of connection needed or awaiting connection in any of our flows. We currently use next_onion_message_for_peer to buffer the message and then dequeue it sending it through lnd. But all the time we do separately the process of trying to connect to peer and then sending messages.

Said so, I'm closing this as is probably our implementation issue.

Add public `reply_path()` method to allow external process or
implementation to access the introduction node information before
responding the onion message.
@a-mpch a-mpch force-pushed the 2025-07-make-reply-path-public branch from cc10483 to 4256944 Compare July 3, 2025 18:24
@a-mpch a-mpch requested a review from jkczyz July 3, 2025 18:26
@valentinewallace valentinewallace removed their request for review July 3, 2025 18:59
@a-mpch a-mpch closed this Jul 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants